Skip to content

Validate that tests don't leaks tracers, scopes and pooled objects#1275

Merged
felixbarny merged 2 commits intoelastic:masterfrom
felixbarny:tests-validate-leaks
Jul 23, 2020
Merged

Validate that tests don't leaks tracers, scopes and pooled objects#1275
felixbarny merged 2 commits intoelastic:masterfrom
felixbarny:tests-validate-leaks

Conversation

@felixbarny
Copy link
Member

@felixbarny felixbarny commented Jul 8, 2020

What does this PR do?

Depends on #1271

With #1270 we have a good foundation to make sure tracers don't leak to other tests classes. This makes GlobalTracer more restrictive so that a tracer can only be set via init that throws if the current tracer is not a noop.

I have also added the recycling tests on the @AfterEach method of AbstractInstrumentationTest which required some adjustments to tests to make sure all transactions and spans are ended and thus recycled after being reported. It also checks the bookkeeper object pool to make sure all objects have returned. This also implicitly tests for scope leaks as those would prevent objects from being recycled.

As a bonus, I've found and fixed a scope leak in the currently unreleased instrumentation of Executor#executeAll.

Checklist

  • This is an enhancement of existing features, or a new feature in existing plugins
    • I have updated CHANGELOG.asciidoc
    • I have added tests that prove my fix is effective or that my feature works
    • Added an API method or config option? Document in which version this will be introduced
    • I have made corresponding changes to the documentation
  • This is a bugfix
  • This is a new plugin
    • I have updated CHANGELOG.asciidoc
    • My code follows the style guidelines of this project
    • I have made corresponding changes to the documentation
    • I have added tests that prove my fix is effective or that my feature works
    • New and existing unit tests pass locally with my changes
    • I have updated CHANGELOG.asciidoc
    • I have updated supported-technologies.asciidoc
    • Added an API method or config option? Document in which version this will be introduced
    • Added an instrumentation plugin? Describe how you made sure that old, non-supported versions are not instrumented by accident.
  • This is something else: Test improvements

@felixbarny felixbarny changed the title Validate that tests don tracer, scope and pooled object leaks in tests Validate that tests don't leaks tracers, scopes and pooled objects Jul 8, 2020
@ghost
Copy link

ghost commented Jul 8, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Branch indexing]

  • Start Time: 2020-07-16T11:39:07.967+0000

  • Duration: 43 min 32 sec

Test stats 🧪

Test Results
Failed 0
Passed 1461
Skipped 11
Total 1472

@felixbarny felixbarny force-pushed the tests-validate-leaks branch from 47639c3 to 881bb14 Compare July 8, 2020 08:56
@SylvainJuge SylvainJuge requested a review from eyalkoren July 8, 2020 11:34
@felixbarny felixbarny force-pushed the tests-validate-leaks branch from 881bb14 to 8968f9b Compare July 8, 2020 15:05
Validate recycling and scope leaks after each test method
Fixes scope leak in Executor#submitAll instrumentation
@felixbarny felixbarny force-pushed the tests-validate-leaks branch from 8968f9b to 5842ad6 Compare July 23, 2020 08:14
@felixbarny felixbarny merged commit 827450d into elastic:master Jul 23, 2020
@felixbarny felixbarny deleted the tests-validate-leaks branch July 23, 2020 09:01
@ghost
Copy link

ghost commented Jul 23, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #1275 updated]

  • Start Time: 2020-07-23T08:15:05.832+0000

  • Duration: 47 min 44 sec

Test stats 🧪

Test Results
Failed 0
Passed 1425
Skipped 11
Total 1436

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants